Skip to content

fix(selectors): resolve env var references at design time for selector context#3446

Merged
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/selector-envvar-resolution
Mar 6, 2026
Merged

fix(selectors): resolve env var references at design time for selector context#3446
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/selector-envvar-resolution

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Selectors now resolve {{ENV_VAR}} references before building context and returning dependency values to consumers
  • Enables env-var-based credentials (e.g. {{SLACK_BOT_TOKEN}}) to work with selector dropdowns
  • Resolves env vars in both useSelectorSetup dependency values and useSelectorOptionDetail detailId

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…r context

Selectors now resolve {{ENV_VAR}} references before building context and
returning dependency values to consumers, enabling env-var-based credentials
(e.g. {{SLACK_BOT_TOKEN}}) to work with selector dropdowns.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 6, 2026 11:45pm

Request Review

@cursor
Copy link

cursor bot commented Mar 6, 2026

PR Summary

Medium Risk
Changes how selector dependency values and detailId are computed and used for query keys/enabled state, which can affect selector dropdown behavior and caching. Scope is small and limited to design-time selector hooks.

Overview
Selectors now resolve {{ENV_VAR}} references before building selector context and returning dependencyValues, using values from useEnvironmentStore.

useSelectorOptionDetail now resolves env-var-based detailId values (and ignores block references) before enabling the detail query and building its queryKey, allowing env-var-backed credentials/IDs to work in selector dropdowns.

Written by Cursor Bugbot for commit 04d1f03. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR fixes env-var-based credentials (e.g. {{SLACK_BOT_TOKEN}}) not working with selector dropdowns by resolving {{ENV_VAR}} template references against the environment store before building selector context and returning dependency values to consumers.

  • use-selector-setup.ts: A new resolvedDependencyValues memo iterates dependencyValues, resolves any {{ENV_VAR}} strings to their real values (or undefined if missing/empty), and replaces the raw dependencyValues in both the selectorContext builder and the hook's return value. The now-redundant isEnvVarReference guard inside the context loop is correctly removed.
  • use-selector-query.ts: A new resolvedDetailId memo in useSelectorOptionDetail applies the same resolution logic to args.detailId before it is forwarded to queryArgs, hasRealDetailId, and the React Query cache key.
  • Both files consistently use || undefined to coerce empty-string env var values to undefined, ensuring unset variables are treated identically across both hooks.
  • Edge cases are handled correctly: missing/empty env vars resolve to undefined and are filtered out before reaching API calls, preventing raw template strings from leaking as credentials.

Confidence Score: 4/5

  • This PR is safe to merge — the changes are focused, well-scoped, and handle edge cases correctly.
  • The env var resolution logic is correct: missing/empty env vars resolve to undefined and are filtered out before reaching API calls, preventing raw template strings from leaking as credentials. Both hooks are consistent in using || undefined. No new security surface is introduced since env vars were already in the client-side environment store. The one point off is the absence of automated tests (the checklist explicitly leaves the test box unchecked), which leaves the fix unverified beyond manual testing.
  • No files require special attention.

Last reviewed commit: 04d1f03

…o context

- Fall back to undefined instead of raw template string when env var is
  missing from store, so the null-check in the context loop discards it
- Use resolvedDetailId in query cache key so React Query refetches when
  the underlying env var value changes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Collaborator Author

@greptile

Align use-selector-setup.ts with use-selector-query.ts by using || instead
of ?? so empty-string env var values are treated as unset.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1
Copy link
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@waleedlatif1 waleedlatif1 merged commit f1efc59 into staging Mar 6, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/selector-envvar-resolution branch March 6, 2026 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant